[r8-obfuscation] Add R8 JNI mapping primitives by simonrozsival · Pull Request #12628 · dotnet/android

simonrozsival · 2026-09-01T14:18:26Z

Summary

Related to #12535

This is layer 1 of 6 in the replacement stack for PR #12575. It extracts the self-contained R8/JNI name-mapping primitives and their focused unit tests so later layers can add build integration and runtime-specific behavior independently.

This layer intentionally changes no build behavior. Nothing invokes these new helpers yet.

What this adds

R8Mapping
- Parses R8/ProGuard mapping files, including class, field, method, constructor, overload, line-range, inline-frame, and class-merging entries.
- Reports the actual source mapping path and line number when file parsing fails.
- Buffers positional records by residual range and obfuscated name as R8 retrace stacks, indexing only the final residual frame as callable so same-class inline-only methods are not rewritten.
- Preserves every original class candidate when R8 merges several classes into one residual class.
- Uses rewrite-manifest class entries to disambiguate reverse class and member mappings, failing closed unless exactly one original class remains allowed.
- Fails closed when reverse field or name-only method lookup remains ambiguous, while allowing a manifest filter to select one candidate safely.
- Records the owning class together with every successful forward member lookup, allowing the resulting manifest to restrict its own reverse mapping safely.
- Tracks accessed entries atomically across concurrent lookups and exposes stable snapshots to manifest consumers.
- Reports a removed declaring class even when reachability input starts with only a required field or method entry, without emitting duplicate class conflicts.
- Provides compatibility and reachability conflict detection used by later stack layers.
- Produces and consumes compact, sorted manifest entries with deterministic \n newlines.
JniDescriptorText
- Validates JNI field and method descriptors, including non-empty slash-separated object type names.
- Rejects malformed object descriptors containing Java source-name dots, array markers inside class names, or empty path segments.
- Rewrites embedded object type names while preserving primitive and unrelated types.
- Converts validated JNI descriptor tokens into Java source type names without redundantly rescanning tokens parsed from a method descriptor.
- Preserves nested binary-name $ separators when converting descriptors to R8 mapping keys.
LdstrRewriter
- Rewrites JNI-sensitive string constants using an IJniNameMapping implementation.
- Handles encoded method and field IDs, constructor descriptors, RegisterNatives entries, multiline registration blocks, and exact JNI class-name strings.
- Leaves unrelated or unresolved strings unchanged.

Test coverage

Adds focused tests for:

R8 mapping syntax, source-aware parse errors, positional inline stacks, qualified and same-class inline frames, residual method preservation, merged-class reverse lookup, manifest disambiguation, zero/multiple-candidate failure, concurrent access tracking and snapshot semantics, member-only owning-class tracking, forward and reverse lookups, overloads, constructors, nested classes, deterministic manifest output, and conflict detection.
JNI method/field descriptor validation, malformed object descriptors, object and array rewriting, Java type conversion, nested $ descriptor-to-mapping-key conversion, and malformed standalone token handling.
JNI-related ldstr forms, including encoded member IDs, constructors, registration blocks, class names, unchanged inputs, and a forward/reverse field round trip restricted by the forward access manifest.

Validation commands:

./dotnet-local.sh test src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Xamarin.Android.Build.Tests.csproj -v minimal --filter 'FullyQualifiedName~R8MappingTests|FullyQualifiedName~JniDescriptorTextTests|FullyQualifiedName~LdstrRewriterTests'
./dotnet-local.sh build src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Xamarin.Android.Build.Tests.csproj --no-restore -v minimal

Result: 82 passed, 0 failed, 0 skipped; test project build succeeded with 0 warnings and 0 errors.

Intentionally out of scope

The following remain for later layers in the replacement stack:

Build integration and MSBuild tasks/targets.
Metadata assembly rebuilding.
Typemap FieldRVA handling.
CoreCLR integration.
NativeAOT integration.
User-facing documentation.

The initial implementation and tests in this layer were reconstructed from the verified PR #12575 source snapshot at ac2b30593ff5afa0659fe35d3de515239c7540fe, followed by descriptor-validation, ambiguity-safety, diagnostics, concurrency, merged-class, inline-frame, and owning-class access-tracking hardening from review. PR #12575 and its branch remain unchanged.

simonrozsival · 2026-09-01T14:29:25Z

/review

github-actions · 2026-09-01T14:30:25Z

✅ Android PR Reviewer completed successfully!

Generated by Android PR Reviewer for #12628

Copilot

Copilot review overview

🟡 Changes recommended

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Lite
Findings: 1 · 2

New issues introduced by this change (3)

Severity	Finding
	`src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/R8Mapping.cs` — ⚠️ operational_implications — `CreateManifestContent()` uses `StringWriter.WriteLine()`, which…
	`src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/JniDescriptorText.cs` — ⚠️ performance — `JniTypeTokenToJavaSource()` uses `System.Linq.Enumerable.Repeat()` to append…
	`src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/R8Mapping.cs` — 💡 suggestion codebase_conventions — New files in this repo typically use file-scoped namespaces…

What changed in this PR

Adds the first “primitive” layer for opt-in R8 JNI name obfuscation by introducing self-contained utilities under Xamarin.Android.Build.Tasks for parsing R8 mapping.txt, validating/rewriting JNI descriptors, and rewriting JNI-sensitive ldstr string forms, along with focused NUnit coverage. This establishes the core mapping/rewriting building blocks so later stack layers can integrate them into the build without changing behavior in this PR.

Changes:

Introduces R8Mapping to parse mapping.txt, support forward/reverse lookups, and generate/consume compact manifest entries.
Adds JniDescriptorText and LdstrRewriter to validate/rewrite JNI descriptors and JNI-name-bearing ldstr payloads.
Adds unit tests covering mapping parsing/ambiguity/conflicts and descriptor/ldstr rewriting behavior.

File	Description
src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/R8Mapping.cs	R8/ProGuard mapping parser + lookup APIs + manifest/conflict helpers.
src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/JniDescriptorText.cs	JNI descriptor/token validation and embedded-class-name rewriting helpers.
src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/LdstrRewriter.cs	Rewrites known JNI-sensitive `ldstr` string forms using an `IJniNameMapping`.
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/JniRemapping/R8MappingTests.cs	NUnit tests for mapping parsing, lookups, ambiguity, and conflict detection.
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/JniRemapping/JniDescriptorTextTests.cs	NUnit tests for descriptor validation, rewriting, and Java-source conversion.
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/JniRemapping/LdstrRewriterTests.cs	NUnit tests for rewriting `ldstr`-encoded member IDs, descriptors, and registration blocks.

github-actions

❌ Reject

Findings: 1 error, 0 warnings.

The parser and descriptor coverage are thorough, but reverse member mapping currently chooses an arbitrary original name for ambiguous R8 mappings, despite the PR’s stated ambiguity-safety guarantee. That can rewrite JNI member IDs to the wrong managed member; reverse field and name-only method lookups should fail closed when more than one candidate remains.

CI build 1576902 is still in progress with no failures reported at review time; the completed Android Tools macOS lane and CLA check passed.

Generated by Android PR Reviewer for #12628 · gpt56 · 111.5 AIC · ⌖ 8.88 AIC · ⊞ 25.7K
_{Comment /review to run again}

simonrozsival · 2026-09-01T15:10:56Z

/review

github-actions · 2026-09-01T15:11:41Z

✅ Android PR Reviewer completed successfully!

Generated by Android PR Reviewer for #12628

github-actions

⚠️ Needs Changes

Found 2 ❌ errors in the R8 parser: reverse class lookup is unsafe when class merging produces duplicate obfuscated names, and same-class inline frames can be indexed as live callable methods. Both can make the later rewrite/reachability pipeline accept an invalid mapping; details and regression-test guidance are inline.

The primitive boundaries, fail-closed member ambiguity handling, manifest snapshots, and focused test organization are otherwise strong. CI build #1576969 is still in progress: five dotnet-android lanes and CLA have passed, Windows/Linux builds are running, and no failure is currently reported.

Generated by Android PR Reviewer for #12628 · gpt56 · 174.9 AIC · ⌖ 9.02 AIC · ⊞ 25.7K
_{Comment /review to run again}

simonrozsival · 2026-09-01T22:49:38Z

/azp run

azure-pipelines · 2026-09-01T22:50:01Z

Azure Pipelines: Successfully started running 1 pipeline(s).

simonrozsival · 2026-09-02T07:47:20Z

/review

github-actions · 2026-09-02T07:48:01Z

✅ Android PR Reviewer completed successfully!

Generated by Android PR Reviewer for #12628

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Require non-empty slash-separated JNI class names and reject Java source names or array markers embedded in object descriptors. Validate standalone type tokens before conversion so malformed input consistently throws ArgumentException. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Fail reverse field and name-only method lookups when multiple allowed original names remain. Make manifest newlines deterministic and avoid LINQ allocation when formatting JNI array types. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Report source paths in mapping parse errors, make access tracking thread-safe with snapshot semantics, remove unused reverse lookup APIs, and avoid redundant JNI token validation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Preserve every original class candidate for R8 class merging and disambiguate reverse mappings through the rewrite manifest. Parse positional mapping groups as retrace stacks so only the final residual frame is callable. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

github-actions

⚠️ Needs Changes

Found 1 warning in the reverse JNI method mapping: primitive descriptor types can be mistaken for residual class names when a custom/applied R8 mapping uses names such as int or void. The inline comment includes the required guard and regression-test shape.

The parser, ambiguity handling, access-manifest tracking, and focused test coverage are otherwise well structured. CI is green.

Generated by Android PR Reviewer for #12628 · gpt56 · 302.6 AIC · ⌖ 8.99 AIC · ⊞ 25.7K
_{Comment /review to run again}

simonrozsival · 2026-09-02T08:21:58Z

/azp run

azure-pipelines · 2026-09-02T08:22:11Z

Azure Pipelines: Successfully started running 1 pipeline(s).

Avoid treating Java primitive source names as package-less residual class names during reverse method lookup. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

## Summary This is layer 2 of 6 in the replacement stack for PR #12575. It adds the internal PE and metadata reconstruction substrate required by later JNI name-rewriting layers, without enabling any build behavior. - Rebuilds managed PE images while preserving existing metadata tokens and supported table row ordering. - Re-emits method bodies, embedded resources, debug-directory data, native resources, and `FieldRVA` mapped data, including ranges from different PE sections. - Preserves overlapping/aliased `FieldRVA` backing data by relocating rewritten fields instead of modifying shared storage. - Validates malformed IL, resource offsets/directories, and strong-name signature directories through controlled rewrite errors. - Emits event/property method semantics explicitly in `HasSemantics` coded-index order. - Rejects unsupported field-backed `ImplMap` rows instead of silently dropping metadata. - Adds low-level metadata encoding, raw-column, IL operand-scanning, and rewrite-plan primitives. - Covers longer and shorter UTF-8 `FieldRVA` replacements, aliased mapped fields, event/property accessor preservation, per-use `#US` splitting, and strong-name signature preservation directly through `AssemblyRebuilder`. - Uses the executing runtime's core assembly version in generated fixtures so rebuilt images load across target frameworks. ## Scope This layer intentionally does **not** add JNI-specific planning or attribute/`ldstr` orchestration, the assembly-rewriter facade, the `RewriteJniNamesForR8` MSBuild task, typemap rewrite behavior, CoreCLR/NativeAOT target integration, or documentation. Those surfaces belong to later stack layers. ## Validation - `./dotnet-local.sh build src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Xamarin.Android.Build.Tests.csproj -c Debug -v:minimal --no-restore` - `./dotnet-local.sh test bin/TestDebug/net10.0/Xamarin.Android.Build.Tests.dll --filter 'FullyQualifiedName~NativeResourceSectionCopierTests|FullyQualifiedName~AssemblyRebuilderTests'` - Direct layer-2 result: 18 passed, 0 failed. - Cumulative layer-3 `RewrittenAssemblyLoadsAndRunsInTheRuntime` result: 1 passed, 0 failed. Related to #12535 Depends on #12628

simonrozsival changed the title ~~Add R8 JNI mapping primitives~~ [r8-obfuscation] Add R8 JNI mapping primitives Sep 1, 2026

simonrozsival marked this pull request as ready for review September 1, 2026 14:29

simonrozsival requested a review from jonathanpeppers as a code owner September 1, 2026 14:29

Copilot AI lite review requested due to automatic review settings September 1, 2026 14:29

simonrozsival mentioned this pull request Sep 1, 2026

[r8-obfuscation] Add PE metadata rebuild substrate #12629

Merged

Copilot started reviewing on behalf of simonrozsival September 1, 2026 14:29 View session

Copilot AI reviewed Sep 1, 2026

View reviewed changes

Comment thread src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/R8Mapping.cs Outdated

Comment thread src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/JniDescriptorText.cs Outdated

Comment thread src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/R8Mapping.cs

github-actions Bot reviewed Sep 1, 2026

View reviewed changes

Comment thread src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/R8Mapping.cs Outdated

github-actions Bot reviewed Sep 1, 2026

View reviewed changes

Comment thread src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/R8Mapping.cs Outdated

Comment thread src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/R8Mapping.cs Outdated

simonrozsival marked this pull request as draft September 1, 2026 15:53

simonrozsival marked this pull request as ready for review September 1, 2026 20:12

simonrozsival added the ready-to-review This PR is ready to review/merge, I think any CI failures are just flaky (ignorable). label Sep 2, 2026

simonrozsival and others added 6 commits September 2, 2026 09:50

Add R8 JNI mapping primitives

39cd8f2

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Address mapping primitives review

eda2d7a

Report source paths in mapping parse errors, make access tracking thread-safe with snapshot semantics, remove unused reverse lookup APIs, and avoid redundant JNI token validation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Track R8 member declaring class access

1263c29

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

simonrozsival force-pushed the simonrozsival-r8-mapping-primitives branch from 51b414b to 1263c29 Compare September 2, 2026 07:53

github-actions Bot reviewed Sep 2, 2026

View reviewed changes

Comment thread src/Xamarin.Android.Build.Tasks/Utilities/JniRemapping/R8Mapping.cs

Preserve primitive types in reverse R8 mapping

ab0cb0a

Avoid treating Java primitive source names as package-less residual class names during reverse method lookup. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

rolfbjarne approved these changes Sep 2, 2026

View reviewed changes

simonrozsival merged commit d270d0c into main Sep 2, 2026
44 checks passed

simonrozsival deleted the simonrozsival-r8-mapping-primitives branch September 2, 2026 11:23

yuseok-kim-edushare mentioned this pull request Sep 2, 2026

R8: emit compiler r8.json into the AAB, and bump 10.0.x pack R8 off 8.11.18 (Play scores 9.0.32, not 8.11.18) #12639

Open

Uh oh!

Conversation

simonrozsival commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What this adds

Test coverage

Intentionally out of scope

Uh oh!

simonrozsival commented Sep 1, 2026

Uh oh!

github-actions Bot commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

❌ Reject

Uh oh!

Uh oh!

simonrozsival commented Sep 1, 2026

Uh oh!

github-actions Bot commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

⚠️ Needs Changes

Uh oh!

Uh oh!

Uh oh!

simonrozsival commented Sep 1, 2026

Uh oh!

azure-pipelines Bot commented Sep 1, 2026

Uh oh!

simonrozsival commented Sep 2, 2026

Uh oh!

github-actions Bot commented Sep 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

⚠️ Needs Changes

Uh oh!

Uh oh!

simonrozsival commented Sep 2, 2026

Uh oh!

azure-pipelines Bot commented Sep 2, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

simonrozsival commented Sep 1, 2026 •

edited

Loading

github-actions Bot commented Sep 1, 2026 •

edited

Loading

github-actions Bot commented Sep 1, 2026 •

edited

Loading

github-actions Bot commented Sep 2, 2026 •

edited

Loading